feat(token-id-capture): resolve each call's parent at request time by ananthsub · Pull Request #2180 · NVIDIA-NeMo/Gym

ananthsub · 2026-07-29T07:14:04Z

Resolves each model call's parent at request time from entries already committed by TokenSink.put. It does not create a second lineage write path.

Multi-worker publication and resolution

sequenceDiagram
    participant H as Harness
    participant W1 as Model worker 1
    participant B as Shared token backend
    participant W2 as Model worker 2

    H->>W1: first request without model-authored history
    W1->>B: resolve(request items)
    B-->>W1: ROOT
    W1->>W1: run inference and stamp ROOT and continuation metadata
    W1->>B: TokenSink.put(TokenEntry)
    B-->>W1: record durable and resolver-visible
    W1-->>H: response A

    H->>W2: continuation echoing response A
    W2->>B: fetch newly committed entry metadata
    W2->>W2: refresh bounded metadata-only index
    W2->>W2: fingerprint lookup and context-digest verification
    alt one token-identity candidate
        W2->>B: lazily load winning call tokens
        B-->>W2: winning TokenEntry and ancestors when needed
        W2->>W2: verify cumulative digest and mark record RESOLVED
    else no safe unique candidate
        W2->>W2: record UNRESOLVED(reason)
    end
    W2->>W2: run inference and stamp the child decision
    W2->>B: TokenSink.put(child TokenEntry)
    W2-->>H: response B

ROOT, RESOLVED, and UNRESOLVED are the only outcome stages. Diagnostic reasons are persisted for analysis but do not weaken reconstruction behavior.

Summary

Persists the request-time resolution outcome, verified parent identity, diagnostic reason, fingerprint version, and compact continuation lookup metadata on each TokenEntry (initial schema version 1).
Keeps TokenSink.put(entry) as the single durable publication boundary. LineageStore is a read-only view over sink-committed entries and has no served-path record() operation.
Extracts IncrementalLineageStore for file, queue, KV, or service adapters. Backends implement entry refresh and lazy load hooks while inheriting Gym's canonical matcher, locking, bounded LRU, and digest checks.
Maintains metadata-only lineage nodes and loads cumulative token arrays only for the winning parent. Cache eviction causes a cold refetch rather than loss of lineage correctness.
Uses a fixed set of striped in-process locks to serialize same-rollout refresh without growing lock metadata per rollout. The file backend also uses the token store's per-rollout flock for cross-process visibility.
Collapses retries only when candidates have identical cumulative token identity. Candidates with different digests remain ambiguous and resolve UNRESOLVED.
Canonically hashes structured request content, tool-call and result identities, and multimodal blocks with versioned, length-delimited fields. Unsupported items and mismatched fingerprint versions fail closed; golden vectors pin the cross-dialect contract.
Reconstructs ROOT as a valid chain start and follows only verified RESOLVED links. UNRESOLVED, a digest conflict, or missing resolution metadata creates a masked fragment. Prefix matching is retained only to recover a recorded RESOLVED link whose direct parent is absent from the current build.
Requires a custom sink to have a resolver over the same backend namespace unless the operator explicitly accepts unresolved continuations. Multi-worker startup also requires process-shared resolver behavior.
Exposes worker-level resolution and capture-failure counters and preserves per-record reasons for offline diagnosis.

Depends on #2341. Required by #2181.

copy-pr-bot · 2026-07-29T07:14:07Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

ananthsub · 2026-08-18T05:31:20Z

/claude review

claude · 2026-08-18T05:45:54Z

SHIP WITH CARE

Request-time parent lineage for token capture: a shared lineage store resolves which recorded call a request continues (fingerprint over model-authored turns, verified by a conversation digest), stamps parent_call_id/cum_len/digest on each TokenEntry, and the builder prefers the verified link over prefix matching. The design is sound and the test coverage is genuinely strong — dialect-agnostic fingerprinting, ambiguous-retry refusal, tree-forks, eviction bounds, and cross-process file resolution are all exercised with assertions on real behavior. The verifier-adjacent contract ("degrade to fallback, never a wrong answer") is the right one, and parent_digest_mismatch correctly quarantines to avoid merging two attempts.

Two findings, both inline, neither a hard blocker but both worth resolving before this drives training data at scale:

builder.py:152 (RISK) — parent_call_id_missing quarantines the node + subtree instead of falling back to _infer_parent, contradicting the module docstring and silently dropping chains the old builder kept (e.g. when the recorded parent was an empty-generation call, which is filtered out of entries before by_call_id is built). Fall back on the missing-parent reason; reserve quarantine for digest-mismatch.
base_responses_api_model.py:1410 (RISK) — the now-default FileLineageStore does read-all + full-file-rewrite + double-fsync per model call, embedding full cumulative_token_ids, giving O(calls² × context) write amplification on the hot path — on the shared/Lustre filesystems store.py already flags as a throughput cliff. And sink.parent_tokens (the only consumer of those persisted tokens) is set but never read. Drop cumulative_token_ids from the file records until a consumer exists; the builder verifies via the TokenEntry digest, not the store.

Also confirm the num_workers>1 guard has been exercised with a real multi-worker rollout, not just unit tests — the CLAUDE.md bar for capture/training changes is real rollouts, and the file-lock + read-after-write assumption across uvicorn workers is exactly the kind of thing green unit tests do not prove.

ananthsub · 2026-08-18T07:52:25Z

/claude review

claude · 2026-08-18T08:05:25Z

Review incomplete: the PR base SHA changed during review (captured merge-base 89b175cfa31ef13444421aeddf0767c6e63d4676, now b37601cc37cee71e153360f5722cf6613b4c1233). The immutable diff I analyzed no longer matches the current PR revision, so ./review-context/check-pr-revision.sh fails the pre-publish check. No inline findings are being posted to avoid anchoring comments to stale lines. Please re-trigger the review against the current head so it runs on an up-to-date diff.

ananthsub · 2026-08-18T08:13:59Z

/claude review

Identifies which recorded call a request continues, from what the harness already sends: hashing the model-authored turns finds the call that produced the last one, and the conversation those turns arrived with is verified before the match is used, so a compacted or rewritten history does not resolve. The record gains parent_call_id, cum_len and digest, and the builder prefers a verified link over inferring one from token prefixes, which makes a chain exact rather than inferred and tells a retry from the call the harness kept. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

…t messages alone The parent check read only the message list. Anthropic sends the system prompt as `system` and the tool schema as `tools`, both siblings of that list, and the chat template renders both into the prompt. A harness that changed either mid-rollout, which Claude Code does on a plan-mode toggle or an MCP attach, still matched, so the call could be handed a prefix rendered under instructions or tools the harness had already replaced. Carry both as a leading pseudo-turn so conversation_digest covers them. Its role is not `assistant`, so the fingerprint the index keys on is unchanged. Tools are normalized to plain data first, since they arrive as dicts on one call and as models on the next and two strings for one schema would break a chain that never changed. Also fail closed when a node has no recorded context digest rather than treating the request as a continuation. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Expose a transport-neutral lineage protocol, use a locked file implementation for Gym-local capture, and require external multi-worker deployments to configure a shared adapter instead of silently losing parent links. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Require adapter implementations to fail closed on ambiguity and make repeated lineage publication idempotent across local and external stores. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Append shared lineage records once and cache only newly published tails in each worker. Fall back to verified token-prefix matching when a recorded parent was filtered from the build, while retaining quarantine for contradictory lineage. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Describe cross-worker consistency, append-only local storage, and the distinct missing-parent and digest-mismatch outcomes with short standalone comments. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Persist root, resolved, and unresolved outcomes at the token publication boundary so multi-worker readers reconstruct only verified lineage. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Use a metadata-only LRU resolver with bounded lock striping, canonical fingerprints, and fail-closed reconstruction so multi-worker continuations resolve without retaining token arrays in memory. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Keep operational comments concise while retaining the failure modes and invariants that explain why the resolver fails closed. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Keep deterministic wire-contract hashes in an allowlisted Python fixture and stamp synthetic rollout records with the explicit root decision required by schema v3. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Exercise the normal custom-sink configuration contract instead of opting collector-only tests into unresolved continuation handling. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Resolve the overlap with terminal attribution without restoring the per-request builder removed by bounded lineage resolution. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

ananthsub · 2026-08-27T22:52:02Z

/claude review

claude · 2026-08-27T23:03:00Z

SHIP WITH CARE — request-time lineage resolution for multi-call token capture.

This is a large, carefully-reasoned change to the training-token-capture path. The correctness logic is sound and the test coverage is genuinely exceptional for a change of this sensitivity: golden wire-vectors pin the fingerprint/digest hashes cross-repo, a conformance kit exercises the sink/source/lineage contracts (including a deliberately-broken backend), and multi-process tests verify cross-worker file visibility. Async hygiene is clean — no httpx.AsyncClient, no ray.get() in async, blocking file reads are wrapped in asyncio.to_thread, and put→resolve visibility is enforced through the store's flock.

What I checked and found solid:

Fail-closed resolution. resolve_parent persists UNRESOLVED on resolver error/absence (sink.py), the builder re-verifies every claimed link by digest and never crosses an unresolved boundary with prefix inference, and the consumer mask now includes unresolved_parent_calls and an empty-delivery guard (consumer.py:189-191). A wrong RESOLVED is caught by digest re-verification downstream.
Dedup / at-least-once. Duplicate identical entries collapse to one call; conflicting payloads for one call id become an unresolved boundary rather than a phantom root (builder.py:224-234).
Schema floor bump to v3. TOKEN_ENTRY_MIN_SCHEMA_VERSION = 3 refuses pre-v3 records. Justified in-comment ("records below schema 3 never left development") — confirm no persisted v1/v2 capture artifacts exist in any live store before merge, since this is a hard read-time reject.
Public-surface removal. per_request builder is deleted from the package __all__ and _BUILDERS. Grep confirms no remaining references (the scenarios_per_request hits are unrelated). Single-response delivery's per_request rejection path is correctly removed alongside it.
Config gate. A custom sink without a lineage store is refused at startup unless allow_unresolved_continuations: true; multi-worker + non-process-shared resolver is rejected. Good operability — fails loud at startup, not silently at train time.

One inline RISK: _request_messages() runs unconditionally on the inference hot path even when capture is disabled. Not a correctness issue; a throughput cost proportional to conversation length. Details inline.

Nothing blocking. The two things to confirm before merge are operational, not code: (1) no live stores hold pre-v3 records, and (2) a real multi-worker rollout has been run end-to-end with a model (per CLAUDE.md — green unit tests alone aren't the bar for capture changes).

Keep the unreleased record contract at schema version one and avoid serializing full conversations when no token-capture context is active. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Stamp terminal-attribution fixtures like current writers and classify unresolved terminal ancestry as broken instead of delivered. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

ananthsub · 2026-08-28T00:39:07Z

/claude review

claude · 2026-08-28T00:50:52Z

SHIP — no reliability blockers found.

Reviewed as a correctness-critical change to the training token-capture / trajectory-reconstruction path (wrong tokens here silently corrupt training data). The design consistently fails safe: every uncertain path resolves to UNRESOLVED and masks the sample rather than guessing, and the offline builder independently re-verifies every claimed parent link by digest (_resolve_parent → parent_digest_mismatch) before reusing any tokens. Duplicate-id conflicts, empty deliveries, and lookup errors all mask. Async correctness is sound — the new resolve_parent on the serving path is gated behind current_capture_context() is not None, offloads file I/O via asyncio.to_thread, and the FileLineageStore reads under the store's shared flock (no httpx, no ray.get, no missing awaits). Multi-worker startup correctly rejects a process-local resolver. Test coverage is genuinely strong: golden cross-repo hash vectors, a conformance kit exercised against two backends + a deliberately-broken one, cross-process file-visibility tests, and dialect-equivalence (Chat/Anthropic/Responses) fingerprint tests.

Non-blocking observations (author's call):

NOTE — per_request builder removed from the public __all__. Technically a public-API removal from nemo_gym.token_id_capture. It had no real consumers (only the internal registry + a consumer path that already rejected it for single-response delivery), and the subsystem is still schema v1 "under development", so blast radius is effectively nil. Flagging only for awareness.
NOTE — missing_resolution vs. the docs. lineage.py/external-agent-harnesses.mdx state prefix inference is "reserved for records written before parent-resolution metadata existed," but _resolve_parent routes a record with parent_resolution=None and parent_call_id=None to missing_resolution → masked, not prefix-inferred. This is unreachable in practice (MIN_SCHEMA_VERSION=1, every current writer stamps a resolution, no sub-v1 records exist) and it fails safe, so it's a comment/doc inconsistency rather than a defect.
NOTE — per-call serving overhead. With the file backend, each captured call now does an incremental JSONL re-index + metadata-only match under to_thread. It's O(new entries) per call / O(total) per rollout and bounded, but worth watching under very high concurrency on long rollouts.

Nothing here requires a change before merge.

Explain when prefix recovery is allowed and make the capture contracts readable without relying on implementation history. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

ananthsub · 2026-08-28T01:12:03Z

Addressed the documentation inconsistency in c703db40b.

Supported records always carry a parent-resolution decision. The builder and external-harness guide now state that prefix matching is used only to recover a RESOLVED link whose direct parent is absent from the frozen build; it never crosses an UNRESOLVED boundary. I also revised the added docstrings and test documentation to describe concrete behavior without relying on implementation history.

This pass did not change runtime logic, so the request-time resolution sequence diagram remains accurate.

ananthsub force-pushed the ananthsub/tokidcap/parent-index branch from ecdccec to 3d810f7 Compare July 29, 2026 12:38

ananthsub changed the base branch from ananthsub/tokidcap/side-calls to ananthsub/tokidcap/delivery July 29, 2026 12:40

ananthsub force-pushed the ananthsub/tokidcap/parent-index branch from 3d810f7 to 2dd3cc3 Compare July 29, 2026 13:14

ananthsub force-pushed the ananthsub/tokidcap/parent-index branch from 2dd3cc3 to 5815504 Compare July 29, 2026 13:41

ananthsub marked this pull request as ready for review July 29, 2026 13:57

copy-pr-bot Bot temporarily deployed to public July 29, 2026 13:58 Inactive

ananthsub requested a review from pthombre July 29, 2026 13:59

copy-pr-bot Bot temporarily deployed to public July 29, 2026 13:59 Inactive

copy-pr-bot Bot temporarily deployed to public July 29, 2026 14:00 Inactive

ananthsub force-pushed the ananthsub/tokidcap/parent-index branch from 5815504 to a9a579d Compare July 29, 2026 16:34

github-actions Bot added the sla:review-overdue Review response is over the one-business-day SLA label Jul 30, 2026

github-actions Bot mentioned this pull request Jul 30, 2026

[Tracker] Pull request handoff SLA #2067

Open

ananthsub force-pushed the ananthsub/tokidcap/parent-index branch from a9a579d to 7219fc9 Compare July 30, 2026 22:19

ananthsub force-pushed the ananthsub/tokidcap/parent-index branch 2 times, most recently from 32711d6 to b117be3 Compare July 31, 2026 01:57

claude Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread nemo_gym/token_id_capture/builder.py

claude Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread nemo_gym/base_responses_api_model.py Outdated

cwing-nvidia mentioned this pull request Aug 22, 2026

chore(hermes_agent): evaluate replacing the fork commit with a named upstream release #2672

Open

7 tasks

ananthsub added 13 commits August 27, 2026 15:23

fix(token-id-capture): define shared lineage consistency

f52ebe7

Require adapter implementations to fail closed on ambiguity and make repeated lineage publication idempotent across local and external stores. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

docs(token-id-capture): clarify shared lineage behavior

5bb5c28

Describe cross-worker consistency, append-only local storage, and the distinct missing-parent and digest-mismatch outcomes with short standalone comments. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

fix(token-id-capture): preserve indexed parent fallback

9f88efd

Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

fix(token-id-capture): make parent resolution explicit

6783330

Persist root, resolved, and unresolved outcomes at the token publication boundary so multi-worker readers reconstruct only verified lineage. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

docs(token-id-capture): preserve lineage rationale

8370fb9

Keep operational comments concise while retaining the failure modes and invariants that explain why the resolver fails closed. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

test(token-id-capture): align lineage fixtures with CI

e2efbca

Keep deterministic wire-contract hashes in an allowlisted Python fixture and stamp synthetic rollout records with the explicit root decision required by schema v3. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

test(token-id-capture): use lineage stub in collector tests

6ea87bd

Exercise the normal custom-sink configuration contract instead of opting collector-only tests into unresolved continuation handling. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

fix(token-id-capture): preserve the lineage-only builder

7e4b464

Resolve the overlap with terminal attribution without restoring the per-request builder removed by bounded lineage resolution. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

claude Bot reviewed Aug 27, 2026

View reviewed changes

Comment thread nemo_gym/base_responses_api_model.py Outdated

ananthsub added 2 commits August 27, 2026 16:18

fix(token-id-capture): address initial schema and hot-path review

ed20a12

Keep the unreleased record contract at schema version one and avoid serializing full conversations when no token-capture context is active. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

test(token-id-capture): align attribution with lineage

24baf15

Stamp terminal-attribution fixtures like current writers and classify unresolved terminal ancestry as broken instead of delivered. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

docs(token-id-capture): clarify lineage guarantees

c703db4

Explain when prefix recovery is allowed and make the capture contracts readable without relying on implementation history. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>

Navigation Menu

Uh oh!

feat(token-id-capture): resolve each call's parent at request time - #2180

feat(token-id-capture): resolve each call's parent at request time#2180
ananthsub wants to merge 16 commits into
mainfrom
ananthsub/tokidcap/parent-index

ananthsub commented Jul 29, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Jul 29, 2026

Uh oh!

ananthsub commented Aug 18, 2026

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Aug 18, 2026

Uh oh!

ananthsub commented Aug 18, 2026

Uh oh!

claude Bot commented Aug 18, 2026

Uh oh!

ananthsub commented Aug 18, 2026

Uh oh!

ananthsub commented Aug 27, 2026

Uh oh!

claude Bot commented Aug 27, 2026

Uh oh!

Uh oh!

ananthsub commented Aug 28, 2026

Uh oh!

claude Bot commented Aug 28, 2026

Uh oh!

ananthsub commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ananthsub commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Multi-worker publication and resolution

Summary

Uh oh!

copy-pr-bot Bot commented Jul 29, 2026

Uh oh!

ananthsub commented Aug 18, 2026

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Aug 18, 2026

Uh oh!

ananthsub commented Aug 18, 2026

Uh oh!

claude Bot commented Aug 18, 2026

Uh oh!

ananthsub commented Aug 18, 2026

Uh oh!

ananthsub commented Aug 27, 2026

Uh oh!

claude Bot commented Aug 27, 2026

Uh oh!

Uh oh!

ananthsub commented Aug 28, 2026

Uh oh!

claude Bot commented Aug 28, 2026

Uh oh!

ananthsub commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ananthsub commented Jul 29, 2026 •

edited

Loading